fix(android): forward-port getReactContext to new arch bridgeless mode #1127
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1127 +/- ##
=======================================
Coverage 77.08% 77.08%
=======================================
Files 32 32
Lines 1727 1727
Branches 579 556 -23
=======================================
Hits 1331 1331
- Misses 343 395 +52
+ Partials 53 1 -52 |
...but do so in a way that is backwards-compatible as the symbols required - do not show up at all until react-native 0.73 - moved to different package names in 0.74
mikehardy
force-pushed
the
reactcontext-in-bridgeless-mode
branch
from
4c5afba to
64dc8fa
Compare
|
Here is the patch if anyone wants to test it: https://github.com/mikehardy/rnfbdemo/blob/rn76/patches/%40notifee%2Breact-native%2B9.1.1.patch diff --git a/node_modules/@notifee/react-native/android/src/main/java/io/invertase/notifee/NotifeeReactUtils.java b/node_modules/@notifee/react-native/android/src/main/java/io/invertase/notifee/NotifeeReactUtils.java
index 4c3b169..549bcf0 100644
--- a/node_modules/@notifee/react-native/android/src/main/java/io/invertase/notifee/NotifeeReactUtils.java
+++ b/node_modules/@notifee/react-native/android/src/main/java/io/invertase/notifee/NotifeeReactUtils.java
@@ -12,6 +12,8 @@ import android.os.Handler;
import android.os.Looper;
import android.util.Log;
import android.util.SparseArray;
+
+import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.lifecycle.Lifecycle;
import androidx.lifecycle.ProcessLifecycleOwner;
@@ -105,11 +107,41 @@ class NotifeeReactUtils {
}
}
+ @SuppressLint("VisibleForTests")
private static @Nullable ReactContext getReactContext() {
- ReactNativeHost reactNativeHost =
+ try {
+
+ // Carefully try to load new architecture classes so we preserve backwards compatibility
+ // These symbols are only available in react-native >= 0.73
+ try {
+ Class<?> entryPoint = Class.forName("com.facebook.react.defaults.DefaultNewArchitectureEntryPoint");
+ Method bridgelessEnabled = entryPoint.getMethod("getBridgelessEnabled");
+ Object result = bridgelessEnabled.invoke(null);
+ if (result == Boolean.TRUE) {
+ Log.d("getReactContext", "We are in bridgeless new architecture mode");
+ Object reactApplication = EventSubscriber.getContext();
+ Method getReactHost = reactApplication.getClass().getMethod("getReactHost");
+ Object reactHostInstance = getReactHost.invoke(reactApplication);
+ Method getCurrentReactContext = reactHostInstance.getClass().getMethod("getCurrentReactContext");
+ return (ReactContext)getCurrentReactContext.invoke(reactHostInstance);
+ } else {
+ Log.d("getReactContext", "we are NOT in bridgeless new architecture mode");
+ }
+ } catch (Exception e) {
+ Log.d("getReactContext", "New Architecture class load failed. Using fallback.");
+ }
+
+ Log.d("getReactContext", "Determining ReactContext using fallback method");
+ ReactNativeHost reactNativeHost =
((ReactApplication) EventSubscriber.getContext()).getReactNativeHost();
- ReactInstanceManager reactInstanceManager = reactNativeHost.getReactInstanceManager();
- return reactInstanceManager.getCurrentReactContext();
+ ReactInstanceManager reactInstanceManager = reactNativeHost.getReactInstanceManager();
+ return reactInstanceManager.getCurrentReactContext();
+ } catch (Exception e) {
+ Log.w("getReactContext", "ReactHost unexpectedly null", e);
+ }
+
+ Log.w("getReactContext", "Unable to determine ReactContext");
+ return null;
}
private static void initializeReactContext(GenericCallback callback) {
@@ -121,7 +153,7 @@ class NotifeeReactUtils {
reactInstanceManager.addReactInstanceEventListener(
new ReactInstanceManager.ReactInstanceEventListener() {
@Override
- public void onReactContextInitialized(final ReactContext reactContext) {
+ public void onReactContextInitialized(@NonNull final ReactContext reactContext) {
reactInstanceManager.removeReactInstanceEventListener(this);
new Handler(Looper.getMainLooper()).postDelayed(callback::call, 100);
}
|
|
This PR did not affect the iOS code thus that CI failure is a flake (and indeed, there is some issue with the simulator even starting the app). Not blocking |
|
This is released as 9.1.2 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
...but do so in a way that is backwards-compatible as the symbols required
Test plan:
I used this script, on my react-native 0.76 testing branch, to verify that it worked:
https://github.com/mikehardy/rnfbdemo/blob/rn76/notifee-demo.sh
I verified it down to react-native 0.72 by changing this line to the various stable versions (0.75.4, 0.74.6, 0.73.10, 0.72.17):
https://github.com/mikehardy/rnfbdemo/blob/1064dcd31a031e9abb53f2611bf32fda5470ee28/notifee-demo.sh#L4
Of particular note is that it contains the patch which I propose here, and is what made things work, with a huge debt of gratitude to @Eclipses-Saros for the initial investigation, idea, and testing. My contribution was making it reflective for backwards compatibility then back-testing it from 0.76.0-rc.6 down to 0.72.17
The test is simply to build the app on android, display a notification, and interact with the notification to make sure events are registered.
The notifee-demo.sh script installs an App that does that - it posts notification and logs the events
Before this patch, the events stopped working with either new architecture enabled on older react-native versions, or with react-native 0.76 in the default configuration (which is new arch enabled + bridgeless)
With the patch, things work everywhere all the time.
Fixes #621
Related #1077